Skip to content

Tighten PDF pagination and cover layout#23

Open
harrisonk0 wants to merge 2 commits intomainfrom
fix/pdf-pagination-polish
Open

Tighten PDF pagination and cover layout#23
harrisonk0 wants to merge 2 commits intomainfrom
fix/pdf-pagination-polish

Conversation

@harrisonk0
Copy link
Copy Markdown
Owner

@harrisonk0 harrisonk0 commented Mar 22, 2026

Summary

  • treat the BB pattern asset as an intentional branded cover panel instead of a faux photo
  • fit more meeting rows on each member detail page before creating continuation pages
  • update the modal page-count estimate to match the denser pagination rules

Testing

  • npm run typecheck
  • npm run build

Summary by CodeRabbit

  • Style

    • Refined spacing, padding, and typography across session reports for improved clarity.
    • Enhanced cover photo area with an overlaid logo and descriptive label for a cleaner presentation.
  • Improvements

    • Improved member-detail pagination so meeting ledgers distribute more predictably across pages.
  • Documentation

    • Clarified a note in the UI docs about PDF renderer styling conventions.

@vercel
Copy link
Copy Markdown

vercel bot commented Mar 22, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
bb-manager Ready Ready Preview, Comment Mar 22, 2026 9:41pm

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 22, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 72720afe-b76b-4418-bf84-278ded877f5c

📥 Commits

Reviewing files that changed from the base of the PR and between 01d9b8d and 84dd34c.

📒 Files selected for processing (4)
  • ARCHITECTURE.md
  • components/SessionReportModal.tsx
  • components/reports/SessionReportDocument.tsx
  • components/reports/reportConstants.ts
✅ Files skipped from review due to trivial changes (2)
  • ARCHITECTURE.md
  • components/reports/reportConstants.ts

📝 Walkthrough

Walkthrough

Extracted member-detail pagination into two constants (MEMBER_DETAIL_FIRST_PAGE_ROWS = 18, MEMBER_DETAIL_CONTINUED_ROWS = 26), replaced hard-coded pagination math with those constants across modal and PDF report components, and applied layout/typography adjustments and a layered cover photo treatment in the session report document.

Changes

Cohort / File(s) Summary
Pagination Constants
components/reports/reportConstants.ts
Added exported constants MEMBER_DETAIL_FIRST_PAGE_ROWS = 18 and MEMBER_DETAIL_CONTINUED_ROWS = 26.
Modal: page-count logic
components/SessionReportModal.tsx
Replaced hard-coded pagination math (previous 14/22 values) with the new constants; restructured estimatedPageCount calculation to handle empty reports explicitly and formatted expression across multiple lines.
PDF report: rendering & styles
components/reports/SessionReportDocument.tsx
Replaced hard-coded slice/chunk sizes for member meeting rows with the new constants; introduced layered cover photo layout (background image + overlay logo/label) and reduced several spacing/typography paddings and sizes across report components.
Docs
ARCHITECTURE.md
Documented that @react-pdf/renderer files are a styling exception using StyleSheet.create() and PDF-specific inline layout values.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 I hopped through rows both bold and neat,
Eighteen to start, then twenty-six repeat,
Numbers tamed to names, the pages hum,
A layered cover, a gentle drum,
I nibble bugs and toast the sum. 🎉

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly summarizes the main changes: tightening PDF pagination (denser member detail rows) and updating the cover layout (branded cover panel with logo overlay).
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/pdf-pagination-polish

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
components/SessionReportModal.tsx (1)

16-17: Extract shared pagination constants to avoid drift

These constants are now duplicated here and in components/reports/SessionReportDocument.tsx. Please move them to one shared module so the estimator and renderer stay locked together.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/SessionReportModal.tsx` around lines 16 - 17, The two duplicated
constants MEMBER_DETAIL_FIRST_PAGE_ROWS and MEMBER_DETAIL_CONTINUED_ROWS should
be extracted into a single shared module and imported where needed so the
estimator and renderer cannot drift; create a small constants module that
exports those names, replace the local definitions in SessionReportModal and the
other file (SessionReportDocument) with imports from that module, remove the
duplicated declarations, and run the build/tests to ensure no references were
missed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@components/reports/SessionReportDocument.tsx`:
- Around line 59-93: Add a documented exception to the project's styling
guideline (update AGENTS.md or ARCHITECTURE.md) stating that components using
`@react-pdf/renderer` (e.g., SessionReportDocument.tsx) must use
StyleSheet.create() and inline styles because the PDF renderer does not support
Tailwind CSS/PostCSS, and therefore such files are exempt from the "Style all UI
with Tailwind CSS via PostCSS" rule; include a short rationale, an explicit note
pointing to `@react-pdf/renderer` usage, and an example mention of
StyleSheet.create to guide future contributors.

In `@components/SessionReportModal.tsx`:
- Around line 66-76: The page-count expression in SessionReportModal.tsx
overestimates when report.members is empty because it always applies Math.max(1,
...) for the member-ledger pages; update the member-ledger term so it returns 0
when report.members.length === 0 (e.g., replace Math.max(1,
Math.ceil(report.members.length / (activeSection === 'junior' ? 14 : 16))) with
a conditional that yields 0 if report.members.length === 0, otherwise
Math.max(1, Math.ceil(...))); keep the rest of the sum (including the reduce
using MEMBER_DETAIL_FIRST_PAGE_ROWS and MEMBER_DETAIL_CONTINUED_ROWS) unchanged.

---

Nitpick comments:
In `@components/SessionReportModal.tsx`:
- Around line 16-17: The two duplicated constants MEMBER_DETAIL_FIRST_PAGE_ROWS
and MEMBER_DETAIL_CONTINUED_ROWS should be extracted into a single shared module
and imported where needed so the estimator and renderer cannot drift; create a
small constants module that exports those names, replace the local definitions
in SessionReportModal and the other file (SessionReportDocument) with imports
from that module, remove the duplicated declarations, and run the build/tests to
ensure no references were missed.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e7db5c15-b9ba-4de6-80ac-98282e926c82

📥 Commits

Reviewing files that changed from the base of the PR and between 397f440 and 01d9b8d.

📒 Files selected for processing (2)
  • components/SessionReportModal.tsx
  • components/reports/SessionReportDocument.tsx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant